-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Remove 7.x versionadded directives #21072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove 7.x versionadded directives #21072
Conversation
.. versionadded:: 7.3 | ||
.. warning:: | ||
|
||
The ObjectMapper component was introduced in Symfony 7.3 as an | ||
:doc:`experimental feature </contributing/code/experimental>`. | ||
The ObjectMapper component is an :doc:`experimental feature </contributing/code/experimental>` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have better suggestion ?
Object Mapper component still experimental
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this proposal too. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component won’t be experimental in 8.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As well as in 7.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we can just remove it, right?
Please let's do it close to the 8.0 release and not now. The reason is, otherwise every upmerge from a new 7.4 feature would make the CI red in 8.0 |
Oskar, can you please explain this a bit? I don't understand why that would happen. Thanks. |
Imagine a 7.4 new feature, with a 7.4 versionadded. If you then upmerge to 8.0 branch, doctor on 8.0 will fail as 7.4 versionadded directives would not be allowed per config anymore, so you would need to manually adjust 8.0 then. I would postpone this close to the 8.0 release. Hope it's clear now |
I disagree with this. If we don't merge this, 8.0 branch will be perpetually red, so we could miss legit errors reported by DOCtor-RST. Although 8.0 doesn't receive new features, but the removal of some deprecations might require to change some contents ... and we could make mistakes there. You are right that when upmerging 7.4 to 8.0 we'll see an error for any new |
Allright then, I am in |
.doctor-rst.yaml
Outdated
@@ -74,12 +74,11 @@ rules: | |||
versionadded_directive_should_have_version: ~ | |||
yaml_instead_of_yml_suffix: ~ | |||
|
|||
# master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed this in 6.4
doctrine.rst
Outdated
|
||
.. _doctrine-entity-value-resolver-resolve-target-entities: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 7.3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a rebase and @xabbuh's comment
@alamirault open to finish this PR? |
6d2b71f
to
3a2bb33
Compare
Thanks Antoine! This was a very nice work to remove all the unneeded stuff from 8.x branch 🥳 |
Note to mergers: we rebased while merging, and the ObjectMapper change was done separately in 8059765 |
More than 275 new features in 7.x cycle !